Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESM Migration #40

Merged
merged 5 commits into from
Sep 9, 2024
Merged

ESM Migration #40

merged 5 commits into from
Sep 9, 2024

Conversation

amydevs
Copy link
Collaborator

@amydevs amydevs commented Sep 9, 2024

Description

This PR aims to complete the ESM migration of the js-ws library.

The biggest change is now the import synax. We now always use .js when referring to any modules we want to import.

Tests

When using fast-check-jest, it uses the @jest/globals package, this is not supported by the swc loader. Hence we need to use patch-package to patch it out: swc-project/plugins#310

When using swc with jest and esm, mocking behaviour is undefined. This is intended: swc-project/swc#5205

Thus all mocks have been removed in the process.

Issues Fixed

Tasks

  • 1. Change to ESM Import Syntax
  • 2. Bump to ESM package versions
  • 3. Fix Benchmarks
  • 4. Fix Tests
  • 5. Remove Mocking
  • 6. Apply Fast-Check Jest Patch

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

fix: `WebSocketServerConfigInput` types were incorrect in `WebSocketServer` constructor

fix: import of `resource-counter` library

fix: consistent import of `ws` library
@amydevs amydevs merged commit abbdf53 into staging Sep 9, 2024
13 checks passed
@CMCDragonkai
Copy link
Member

It looks like we will be switching to DI instead of mocking for most things then...

@CMCDragonkai
Copy link
Member

Date mocking would be complex in the keys domain of PK though, but it should still be doable, we'll cross that bridge when we get there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants